Add Weekly Financial Report functionality with repository and tests - #65

workers/main/src/services/WeeklyFinancialReport/index.ts (1)

1-1: Re-export the interface for downstream consumers

Only the repository class is re-exported. Call-sites that depend on the GenerateReportInput type or the IWeeklyFinancialReportRepository interface will still have to reach into the sub-path, defeating the purpose of an index barrel.

 export * from './WeeklyFinancialReportRepository';
+export * from './IWeeklyFinancialReportRepository';

workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts (1)

50-63: Broaden test assertions to validate numeric accuracy

The test only checks that strings are non-empty and contain certain keywords. A regression could silently alter the underlying maths (e.g., margin calculation) yet still pass. Consider asserting at least one concrete figure (e.g., expected total revenue for Group A).

workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)

108-108: Legend string has unbalanced spacing

Minor formatting nit: double spaces after the yellow-circle entry look like a typo.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (2)

33-44: Inject currentDate for deterministic tests

Prior review feedback highlighted that grabbing new Date() internally hinders reproducible tests. Accept it as an optional parameter with a default to keep runtime ergonomics unchanged.

-  async generateReport({
-    targetUnits,
-    employees,
-    projects,
-  }: GenerateReportInput) {
-    const currentDate = new Date();
+  async generateReport(
+    { targetUnits, employees, projects }: GenerateReportInput,
+    currentDate: Date = new Date(),
+  ) {

---

48-62: Quadratic complexity caused by repeated filtering

Inside the loop we call GroupAggregator.aggregateGroup, which performs Array.filter over all targetUnits for every distinct group. With g groups and n units this degrades to O(n × g) (worst-case O(n²)).

Pre-group once with a Map:

-    for (const targetUnit of targetUnits) {
-      this.processTargetUnit({ … });
-    }
+    const grouped = new Map<number, TargetUnit[]>();
+    for (const unit of targetUnits) {
+      (grouped.get(unit.group_id) ?? grouped.set(unit.group_id, []).get(unit.group_id)!).push(unit);
+    }
+
+    for (const [_, groupUnits] of grouped) {
+      this.processTargetUnit({
+        targetUnit: groupUnits[0], // representative for meta‐data
+        targetUnits: groupUnits,
+        … // rest unchanged, drop processedGroupIds
+      });
+    }

You’ll also be able to delete the processedGroupIds set entirely.

workers/main/src/services/WeeklyFinancialReport/GroupAggregator.ts (1)

3-15: Prefer a plain function over a static-only class

The class hosts only one static method and no state. Keeping it as a class introduces needless ceremony and violates the no-static-only-class rule flagged by Biome. A small utility function exported from the module would be simpler to tree-shake and test.

-export class GroupAggregator {
-  static aggregateGroup(targetUnits: TargetUnit[], targetUnitId: number) {
-    …
-    return { groupUnits, groupTotalHours };
-  }
-}
+export function aggregateGroup(
+  targetUnits: TargetUnit[],
+  targetUnitId: number,
+) {
+  …
+  return { groupUnits, groupTotalHours };
+}

Update the import sites accordingly.

workers/main/src/services/WeeklyFinancialReport/MarginalityCalculator.ts (2)

19-47: Static-only class & this usage can be flattened

Same as above, the calculator is purely functional. You can export three top-level functions (calculate, classify, getIndicator) and the enum. This removes the surprising this.classify/this.getIndicator calls that Biome warns about and avoids potential confusion when the class name is minified.

---

36-45: Redundant case MarginalityLevel.Low

Because the default branch already returns the same value, the explicit Low case is superfluous.

-      case MarginalityLevel.Low:
-        return ':arrowdown:';
-      default:
-        return ':arrowdown:';
+      default:
+        return ':arrowdown:';

workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts (2)

7-12: Interface name violates PascalCase convention

formatSummaryInput is the only interface in the codebase that starts with a lowercase letter; this stands out and breaks TS/ESLint naming rules.

-export interface formatSummaryInput {
+export interface FormatSummaryInput {

Remember to update the type at the call-site in formatSummary.

---

25-41: Static field with arrow function is unnecessary

Declaring static formatDetail = (...) => creates a class field and incurs an extra property lookup compared with a regular static method. Change to a standard static method unless you intentionally need lexical this.

-  static formatDetail = ({
+  static formatDetail({
     …
   }: FormatDetailInput) {
     return (
       …
     );
-  =>
+  }

workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)

153-170: Date arithmetic is hard to reason about

The opaque magic numbers - ((getDay() + 6) % 7) - 7 make maintenance tricky. Consider extracting this into a small helper getPreviousWeekRange(date) that clearly communicates intent and can be unit-tested.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro